chore: rework CSS extraction#277
Merged
layershifter merged 4 commits intomicrosoft:mainfrom Nov 30, 2022
Merged
Conversation
fc37b33 to
a39eac7
Compare
📊 Bundle size report🤖 This report was generated against e242ccaaa99b08bbb0cfdfdb9fcd2db802c33fc2 |
4e9ed2a to
35cb4c8
Compare
35cb4c8 to
86554e5
Compare
layershifter
commented
Nov 24, 2022
| "root": "e2e/nextjs", | ||
| "sourceRoot": "e2e/nextjs/src", | ||
| "projectType": "library", | ||
| "implicitDependencies": ["@griffel/webpack-loader"], |
Member
Author
There was a problem hiding this comment.
As we don't import/require @griffel/webpack-loader anywhere - Nx fails to detect it as a dependency.
layershifter
commented
Nov 24, 2022
| "targets": { | ||
| "test": { | ||
| "executor": "@nrwl/workspace:run-commands", | ||
| "dependsOn": [{ "target": "build", "projects": "dependencies" }], |
Member
Author
There was a problem hiding this comment.
Have been in a wrong spot 🤦
ling1726
reviewed
Nov 25, 2022
| throw new Error('Failed to find and entry points in "compilation.entrypoints"'); | ||
| } | ||
|
|
||
| const mainEntryPoint = entryPoints[0]; |
Contributor
There was a problem hiding this comment.
is the main entrypoint guaranteed to be the first entrypoint in compilation.entrypoints ?
Contributor
There was a problem hiding this comment.
actually, what if webpack is configured with multiple entrypoints?
Member
Author
There was a problem hiding this comment.
It's a good question. In all scenarios that I currently tested attaching to main entrypoint was enough. So, I don't have answer if it's good or bad. Currently works.
ling1726
reviewed
Nov 25, 2022
packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
Outdated
Show resolved
Hide resolved
ling1726
reviewed
Nov 25, 2022
packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
Outdated
Show resolved
Hide resolved
ling1726
reviewed
Nov 25, 2022
packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
Outdated
Show resolved
Hide resolved
ling1726
approved these changes
Nov 25, 2022
…gin.ts Co-authored-by: ling1726 <[email protected]>
…gin.ts Co-authored-by: ling1726 <[email protected]>
…gin.ts Co-authored-by: ling1726 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors Webpack plugin for CSS extraction.
Before
We use the same approach as CompiledCSS: we create a new
cacheGroupinoptimization.splitChunksand force all generated CSS files by us to go there.As a result we will get a single asset (CSS file) that we can process and sort rules. This works, but this approach requires
optimization.splitChunksto be enabled. One of our key partners has this setting disabled in dev environment: all try outs to enable it were not successful.This approach also prevents all attempts to implement chunks splitting support and avoid a single CSS file.
After
TL;DR Does the same, works differently ¯_(ツ)_/¯
The key difference is that we don't use
optimization.splitChunksand moving CSS files to a target chunk.Initial attempt
Inspired by atlassian-labs/compiled#724. All processing was done during
.processAssetsstage and it worked: we got a CSS file per a chunk (if it containedmakeStylescalls) and I was able to extract rules from it to a dedicated chunk.But:
compilation.deleteAsset()) asmini-css-extract-pluginwill register async chunks in JS runtime and these assets will fail to load..runtimeRequirementInTree(undocumented Webpack hook) otherwise chunks that have empty CSS could be registered in JS runtime 💣Implementation in this PR
After multiple attempts to do something with assets I realized that
.processAssetsis too late to transfer CSS between chunks. The problem that at any stage before.runtimeRequirementInTreewe don't have assets - we operate with modules (not really well documented). Simple explanation: an asset is a concatenated set of modules.On
.afterChunks(again undocumented, happens after a chunk graph gets generated and modules were collected):mini-css-extract-pluginto that chunk (if they contain CSS produced by Griffel)On
.processAssetswe are in the same condition actually as currently: all CSS modules produced by Griffel were moved to a dedicated chunk -> this chunk has a single CSS file -> we sort rules in it ✅